Report usages of the deprecated 'ext.loadimpact' option #4084
Conversation
joanlopez
force-pushed
the
report-loadimpact
branch
from
8c8bdf7 to
cd20b81
Compare
| if err := lct.initRunner.SetOptions(configToReinject); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Here, where we get the consolidated options, is where we check if any |
There was a problem hiding this comment.
I added it here because it looks like a considerably safe place among all the places configuration is read/written and manipulated in general, but any suggestion will be more than welcome.
There was a problem hiding this comment.
Also, note that this will be reported despite it's a local (k6 run) or a remote execution (k6 cloud), but I guess it's fine because a script that holds ext.loadimpact is still a candidate of experiencing issues if we remove such support.
In any case, if we want to get more accurate metrics (only cloud executions), we can always account reports with this and a test run id.
| // Here, where we get the consolidated options, is where we check if any | ||
| // of the deprecated options is being used, and we report it. | ||
| if _, isPresent := configToReinject.External["loadimpact"]; isPresent { | ||
| if err := lct.preInitState.Usage.Strings("deprecated_options", "ext.loadimpact"); err != nil { |
There was a problem hiding this comment.
In https://github.com/grafana/k6/pull/4030/files the key is deprecations/<what is deprecated>. I would prefer if we keep the keys more generic and deprecated_options is very specific to options by the looks of it and I do not know of any other option that will make it relevant to have a separate key.
There was a problem hiding this comment.
So, is your suggestion: deprecations/ext.loadimpact?
I'm happy with that one as well, sure!
There was a problem hiding this comment.
I guess that looks okay - yes. Don't know if we want options somewhere in there, but as a one of case this seems good enough.
What?
It adds a new
deprecated_optionsfield into the usage reports, and accounts ifext.loadimpactis still being used, and how often.Why?
Because we likely will remove that option parameter before v1, so we need to make some plans.
Checklist
make lint) and all checks pass.make tests) and all tests pass.Related PR(s)/Issue(s)
Related with https://github.com/grafana/k6-cloud/issues/2210